Skip to content

Adding drop down to diff table#4354

Merged
carolynqu merged 13 commits into
flutter:masterfrom
carolynqu:diff-dropDown
Aug 10, 2022
Merged

Adding drop down to diff table#4354
carolynqu merged 13 commits into
flutter:masterfrom
carolynqu:diff-dropDown

Conversation

@carolynqu
Copy link
Copy Markdown
Contributor

@carolynqu carolynqu commented Aug 5, 2022

For deferred apps, in the diff-table, the tab auto-opens to "Entire app" with a dropdown to switch between the entire app, the main unit, and the deferred units:

diffDropDown.mp4

@carolynqu carolynqu requested a review from elliette August 5, 2022 19:44
Comment thread packages/devtools_app/lib/src/screens/app_size/app_size_controller.dart Outdated
Comment thread packages/devtools_app/lib/src/screens/app_size/app_size_controller.dart Outdated
@carolynqu carolynqu marked this pull request as ready for review August 9, 2022 16:33
@elliette
Copy link
Copy Markdown
Member

elliette commented Aug 9, 2022

Let's add a test case for the diff view with deferred units (it should be pretty similar to the test here: #4332)

Comment thread packages/devtools_app/test/app_size/app_size_screen_test.dart Outdated
if (currentTab.key == AppSizeScreen.analysisTabKey &&
isDeferredApp)
_buildAppUnitDropdown(),
if (isDeferredApp) _buildAppUnitDropdown(currentTab.key!),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add some spacing here:

if (isDeferredApp) ...[
  _buildAppUnitDropdown(currentTab.key!),
  const SizedBox(width: defaultSpacing),
],

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added spacing so that it only impacts the visuals when both drop downs are there.

_diffCallGraphRoot.value = null;
_oldDiffCallGraph = null;
_newDiffCallGraph = null;
_isDeferredApp.value = false;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's split this into a separate PR so we can add a test for it at the same time

@carolynqu carolynqu merged commit ba21e65 into flutter:master Aug 10, 2022
@carolynqu carolynqu deleted the diff-dropDown branch August 10, 2022 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants